Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support loading animations with BOM #2167

Merged
merged 2 commits into from
Jul 14, 2022
Merged

Support loading animations with BOM #2167

merged 2 commits into from
Jul 14, 2022

Conversation

mattleibow
Copy link
Contributor

Description of Change

The JSON parser inside skia does not support BOMs, so we have to do it.

Bugs Fixed

API Changes

None.

Behavioral Changes

None.

Required skia PR

None.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Merged related skia PRs
  • Changes adhere to coding standard
  • Updated documentation

return Create (stream, stream.Length);
return Create (stream, stream.Length - stream.Position);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bug I found when you pass a stream that was not at 0. This can be used to create a data object from a section of a stream.

Comment on lines +22 to +29
if (len >= 2 && buffer[0] == 0xfe && buffer[1] == 0xff)
return 2;
else if (len >= 3 && buffer[0] == 0xef && buffer[1] == 0xbb && buffer[2] == 0xbf)
return 3;
else if (len >= 3 && buffer[0] == 0x2b && buffer[1] == 0x2f && buffer[2] == 0x76)
return 3;
else if (len >= 4 && buffer[0] == 0 && buffer[1] == 0 && buffer[2] == 0xfe && buffer[3] == 0xff)
return 4;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BOM detection. Right now just the basic UTF-7/8/16/32

Comment on lines -59 to +64
animation = GetObject (SkottieApi.skottie_animation_make_from_stream (stream.Handle));
return animation != null;
using var data = SKData.Create (stream);
return TryCreate (data, out animation);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All animations are now created through the SKData overload, which we then use to detect the BOM.

We could use the stream, but then we have to use buffering and all other manner of complex mechanisms. This way we load the full data, and then optionally skip 4 bytes before passing it to C++.

@mattleibow mattleibow merged commit 8f81be2 into main Jul 14, 2022
@mattleibow mattleibow deleted the dev/bom branch July 14, 2022 03:26
@mattleibow mattleibow added this to the v2.88.1 milestone Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BUG] Skottie cannot load animations with a BOM
1 participant